-
Notifications
You must be signed in to change notification settings - Fork 70
chore: upgrade to go 1.24 for tensorboard-controller #755
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
/assign @mkoushni |
f510725 to
ab3a5d0
Compare
liavweiss
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, Marina. In addition to the review comments, please include the go.sum file in the PR as well.
This file is important for validating module dependencies and ensuring consistent, reproducible builds across different environments.
ea6177c to
93b0e3d
Compare
andyatmiami
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @mkoushni - appreciate your changes.
We are very close - given the poor (initial) state of the Dockerfile prior to your changes - just using this PR as a means to right those wrongs.
Apply those changes (and ask any questions as you see fit!) - but imagine we can get this merged fairly quickly after the Dockerfile changes are delivered.
93b0e3d to
40494d2
Compare
|
@andyatmiami fixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
thanks @mkoushni for this contribution.
verified changes by updating tensorboard-controller in a local kind cluster
- cluster setup script:
- update script
- verification script:
Also logged into Central Dashboard and verified common Tensorboard activities completed successfully (*) and no errors surfaced in logs:
- create new Tensorboard
- delete Tensorboard
* I was not able to actually launch the Tensorboard app within the container due to emulation/arm64 issues. I did however collab with @mkoushni (who has an amd64 system) and verified the actual TB app loaded appropriately.
|
/ok-to-test |
Signed-off-by: Marina Koushnir <[email protected]> Signed-off-by: Mathew Wicks <[email protected]>
40494d2 to
60da2e6
Compare
Signed-off-by: Mathew Wicks <[email protected]>
Signed-off-by: Mathew Wicks <[email protected]>
|
@mkoushni I fixed the messed up commit history, please avoid using merge commits in any PRs unless you know it's required for a specific case. I also fixed the fact that Additionally I ran |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: thesuperzapper The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
9f95e36
into
kubeflow:notebooks-v1

Goal
This PR updates the required Go version for the tensorboard-controller component to Go 1.24.x. This change is necessary to maintain compatibility with updated Kubernetes client libraries and ensure the component is built with current, supported security patches.
Changes Made
Updated the go.mod file to specify go 1.24.
Synchronized dependencies via go mod tidy and go mod vendor to lock in compatible library versions (including necessary updates to transitive dependencies like golang.org/x/text, etc.).